Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

probe: Fixed premature firing of 'homing:homing_move_begin' #6779

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nickweedon
Copy link

When probing multiple points such as when calibrating a bed mesh, the ProbePointsHelper start_probe method initiates a horizontal repositioning move and then immediately begins to probe without waiting for the repositioning move to complete. This causes the 'homing:homing_move_begin' to trigger before the Z direction move has even began. This is problematic when running BED_MESH_CALIBRATE with [homing_heaters] configured since it means that the bed will never attempt to re-heat.

This fix addresses this issue simply by waiting for the moves to complete before begining the actual probe.

When probing multiple points such as when calibrating a bed mesh, the
ProbePointsHelper start_probe method initiates a horizontal repositioning
move and then immediately begins to probe without waiting for the
repositioning move to complete. This causes the 'homing:homing_move_begin'
to trigger before the Z direction move has even began.
This is problematic when running BED_MESH_CALIBRATE with [homing_heaters]
configured since it means that the bed will never attempt to re-heat.

Signed-off-by: Nick Weedon <[email protected]>
nickweedon added a commit to nickweedon/klipper that referenced this pull request Jan 5, 2025
This PR allows bed mesh calibration to work well with probes that are
sensitive to EMI coming from the bed heater while still being able to
maintain the bed temperature.
This works through adding an additional configuration setting to the
```[homing_heaters]``` section that will cause homing to wait for the
heaters to re-heat if they are outside the threshold AFTER homing has
occurred.
Note that in order to work properly this feature PR relies on the bugfix
in PR Klipper3d#6779.

Signed-off-by: Nick Weedon <[email protected]>
nickweedon added a commit to nickweedon/klipper that referenced this pull request Jan 5, 2025
This PR allows bed mesh calibration to work well with probes that are
sensitive to EMI coming from the bed heater while still being able to
maintain the bed temperature.
This works through adding an additional configuration setting to the
```[homing_heaters]``` section that will cause homing to wait for the
heaters to re-heat if they are outside the threshold AFTER homing has
occurred.
Note that in order to work properly this feature PR relies on the bugfix
in PR Klipper3d#6779.

Signed-off-by: Nick Weedon <[email protected]>
nickweedon added a commit to nickweedon/klipper that referenced this pull request Jan 6, 2025
This PR allows bed mesh calibration to work well with probes that are
sensitive to EMI coming from the bed heater while still being able to
maintain the bed temperature.
This works through adding an additional configuration setting to the
```[homing_heaters]``` section that will cause homing to wait for the
heaters to re-heat if they are outside the threshold AFTER homing has
occurred.
Note that in order to work properly this feature PR relies on the bugfix
in PR Klipper3d#6779.

Signed-off-by: Nick Weedon <[email protected]>
@KevinOConnor
Copy link
Collaborator

Thanks. This change seems like it would cause significant delays during probe sessions (as wait_moves() takes a long time to complete). I'm not sure I understand the problem being reported - why does homing_heaters care where the toolhead is when homing nominally starts?

-Kevin

@nickweedon
Copy link
Author

Thanks for checking this out!
So homing_heaters cares about this because it needs to know when the actual BLTouch probe is about to begin as opposed to when the toolhead is simply being horizontally re-positioned. Otherwise it will keep the heaters turned off during the entire movement from the previous probe to the next probe. The impact is quite apparent as it means that the entire bed mesh probing operation becomes one giant chained sequence of probes where the bed heater is never re-enabled for more than a few milliseconds and thus the bed will usually be cold by the end of bed mesh calibration.

Here is an example of the old behavior:

Turn off heater -> probe -> turn on heater -> turn off heater -> probe -> turn on heater -> turn off heater -> move to next probe position and probe -> turn on heater -> turn off heater -> ...

New behavior (bed will be reheated between toolhead horizontal moves):
Turn off heater -> probe -> turn on heater -> turn off heater -> probe -> turn on heater -> move to next probe position -> turn off heater -> probe -> turn on heater -> turn off heater -> ...

Note that I have not observed any slowdowns from 'wait_moves()' since it the printer is waiting on the horizontal movement but its quite possible i overlook/misunderstand something here?

As an aside, this fix is also necessary for my related bed reheating feature PR to work #6780

@nefelim4ag
Copy link
Contributor

I did something like this to test:

[homing_heaters]
steppers: stepper_z
heaters: heater_bed

And tested the bed mesh sequence without and with patch:
image

I don't think this is the right way to work around that behavior.

As an example, that is my bed_mesh config:

[bed_mesh]
horizontal_move_z: 3
mesh_min:  20, 5
mesh_max: 380, 355
speed: 1000
probe_count: 7, 7

So 380/7 = 54 mm maximum distance between probes.
By using the Prusa calculator
With cruise ratio = 50%, move profile will look like this:
image

Let's roughly estimate travel time to move with that speed, assuming half of the time it will accelerate or deaccelerate.
54 / 2 / 400 + 54 / 2 / 200 = 0.2s this is barely enough to enable and disable heaters - heaters always scheduled 300ms ahead.

I tried to limit the speed to 50 mm/s, so I have around 1s between probes, and there is enough time to enable and disable them.

Looks like there are just no events that will trigger heaters enabled/disabled, and self.printer.lookup_object('toolhead').wait_moves() works as a hack.

@nickweedon
Copy link
Author

nickweedon commented Jan 20, 2025

Thanks for checking out this change @nefelim4ag.

So just to point something out here, irrespective of whatever algorthm you use and given current bed heater technology, if you are spending the majority of the time probing as opposed to anything else and you are also wishing to turn off the bed to avoid EMI interferance, then you will always have the bed go cold.

There is no free lunch here, it is impossible to keep the bed hot while running at the kind of speed and acceleration you are using and in fact even if you were to lower the speed significantly it may still not keep the bed at the desired temperature. If you are looking for this behavior then see my feature PR #6780. Apologies if this was unclear as I was using bed heating and probing as an example of the kind of behavior this bug fix addresses but this change is to fix a bug in the probing event logic, not specifically bed heating.

With regard to 'wait_moves', i'm really not sure why you refer to this as a hack as this is acutally the correct behavior. Repositining the toolhood should not be considered part of probing. This is part of the mesh calibration and it is illogical to conflate these things and it is in essence a bug. Please feel free to elaborate though if I perhaps overlook something here with respect to 'wait_moves'.

Just to further re-iterate, the bug fix is actually not supposed to address bed heating in particular. Technically this bug fix has nothing to do with bed heating, just the probing event which is logically decoupled from bed heating via an observer pattern. This event could actually be consumed by all kinds of different code for various reasons and cause similar bugs in those cases.

If you take a look at #6780 then you will see the actual solution that I have for bed heating. It can actually work independantly of this bug fix but i think it makes sense to fix this underlying issue as this will make for a bit more of an optimized solution when combined with #6780. I believe the #6780 feature is the only way possible to solve for the bed reheating scenario.

@Sineos
Copy link
Collaborator

Sineos commented Jan 20, 2025

While I cannot comment on how the proposed code is working, I do agree with the intention behind it.

When using the [homing_heaters] functionality, the temperature drops for both the bed and the extruder are quite significant, as they stay off for longer periods, such as during Quad Gantry Levelling or adaptive meshing.

I'd also appreciate an improvement here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants